Skip to content

Conversation

lindblandro
Copy link

The clock-reference property is marked as required in the bindings file. Without these changes creating new boards based on stm32u5 either requires explicitly disabling otghs_phy or setting the clock-reference -property. The in-tree boards all use the same value for it, which is probably a sensible default.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Selecting SYSCFG_OTG_HS_PHY_CLK_16MHz in these board is related to the HSE clock being at 16MHz. For a new board that would have another HSE frequency, this value would need to be updated among the supported ones, or PHY source clock be changed (OTGHS_SEL(0)).

It makes sense usbhs_phy node is default disabled at SoC DTSI level, but I don't think the SoC DTSI can set a default clock-reference value, it should remain at board DTS level.

Copy link
Contributor

@mathieuchopstm mathieuchopstm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not both (clock-reference = + status = "okay")? 🙂

Making the property required: true but assigning a default is arguably worse than not having required: true: it doesn't cause build failures if one forgets to set the property.

@etienne-lms
Copy link
Contributor

I agree, the board should set both clock-reference = .. + status = "okay".

@lindblandro lindblandro force-pushed the stm32u5-dts-disable-otghs-phy branch from 8ba4935 to 595fdff Compare October 8, 2025 16:04
@lindblandro
Copy link
Author

Why not both (clock-reference = + status = "okay")? 🙂

Making the property required: true but assigning a default is arguably worse than not having required: true: it doesn't cause build failures if one forgets to set the property.

Yeah that makes a lot more sense. 👍

@lindblandro
Copy link
Author

Selecting SYSCFG_OTG_HS_PHY_CLK_16MHz in these board is related to the HSE clock being at 16MHz. For a new board that would have another HSE frequency, this value would need to be updated among the supported ones, or PHY source clock be changed (OTGHS_SEL(0)).

It makes sense usbhs_phy node is default disabled at SoC DTSI level, but I don't think the SoC DTSI can set a default clock-reference value, it should remain at board DTS level.

Fixed this.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I would however remove this sentence from the commit message. I think it may be a source of confusion.

The in-tree boards all use the same value for it, which is probably a
sensible default.

The clock-reference property is marked as required in the bindings file.
Without these changes creating new boards based on stm32u5 either requires
explicitly disabling otghs_phy or setting the clock-reference -property.

Signed-off-by: Henrik Lindblom <[email protected]>
@lindblandro lindblandro force-pushed the stm32u5-dts-disable-otghs-phy branch from 595fdff to fa4a4f3 Compare October 9, 2025 06:59
Copy link

sonarqubecloud bot commented Oct 9, 2025

@cfriedt cfriedt merged commit 0c21149 into zephyrproject-rtos:main Oct 10, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants